fix: differentiate between array types and standard structs #134
Conversation
|
Was able to drop the need to recurse twice on request bodies by passing a the |
| @@ -67,6 +161,8 @@ func TestServer_generateOpenAPI(t *testing.T) { | |||
| require.NotNil(t, document.Paths.Find("/")) | |||
| require.Nil(t, document.Paths.Find("/unknown")) | |||
| require.NotNil(t, document.Paths.Find("/post")) | |||
| require.Equal(t, document.Paths.Find("/post").Post.Responses.Value("200").Value.Content["application/json"].Schema.Value.Type, "array") | |||
| require.Equal(t, document.Paths.Find("/post").Post.Responses.Value("200").Value.Content["application/json"].Schema.Value.Items.Ref, "#/components/schemas/MyStruct") | |||
There was a problem hiding this comment.
Need to do more to validate the actual schema reference here
|
@EwenQuim when you have a moment to do you mind taking a look. |
| if v == nil { | ||
| return "unknown-interface" | ||
| s.getOrCreateSchema("unknown-interface", struct{}{}) |
There was a problem hiding this comment.
Need test to ensure an unknown-interface component schema is being added to the spec via test. I did not pick this up until my own diff
|
I'll look at it omorrow morning, thanks for this investigation. It is indeed complex. |
Ignore this error as it is not possible with the current instationation of of the openapi generator used. Unless cyclic err is set or a custom schema customizer is provided it is not possible to error.
…lect the struct and not it's array/ptrs
|
Sorry for the spam 😓. Messed some things up while getting my fork ready for a build. |
|
@EwenQuim any word on this? |
|
Ready to review it, I'd like to merge #137 just before this one, so I'll be able to work on Multi Return OpenAPI spec |
EwenQuim
changed the title
|
Mmmh maybe I'll merge this before #137 finally... |
| case reflect.Slice, reflect.Array: | ||
| item := dive(s, t.Elem(), tag, maxDepth-1) | ||
| tag.name = item.name | ||
| tag.Value = &openapi3.Schema{ | ||
| Type: "array", | ||
| Items: &item.SchemaRef, | ||
| } | ||
| return dive(t.Elem(), maxDepth-1) | ||
| return tag |
There was a problem hiding this comment.
Perfect, you really dived into the code! 😉
This did turn out to be pretty complex.
The following is by no means finished. More looking for a discussion. If you take a look #133 the bug is fairly clear. We do not respect response nor return types when they are arrays.
The only way I can think of solving this is that we must create the ref to the schema and link it at the bottom of a recursion tree. If you do not do this you will end up with improper refs, with the original approach the input for the first type wins so if an array of
MyStructis passed the schema will be a list ofMyStructrather than just the simple Schema ofMyStruct. SeeschemaDive. Currently this does require us to dive twice on request bodies. Since we seem to check request bodies if they areunknown-type/string, which I am unsure why (wouldn't we want to have []string be a valid requestBody)? As long as that check exists I'm not sure how to avoid both recursive steps.This does however give the proper diff
This does need more tests, but I'd like another pair of eyes too look at the approach @EwenQuim if you don't mind. Could you please take a look. I'm sure this can be simplified. I've been staring at it too long at this point.
@epentland feel free to chime since you brought this to our attention
Thank you